[RFC 0097] Unset read permission bit on /nix/store for other users #97
Conversation
|
Fixed the parts about usage activity and added the drawback about tools needing more permissions. |
|
I rewrote a lot of the text. |
…re for other users
|
I once tried removing read permission for the nixbld group (NixOS/nixpkgs@224d0d5) because it gives some of the advantages of build sandboxing without needing a mount namespace. Unfortunately it broke our VM tests but that should be fixable. I'm not opposed to this proposal, but it does have a "security through obscurity" vibe. Currently, you shouldn't store secrets in the store, and because Unix software has not traditionally treated paths as secrets, it would be dangerous to rely on the secrecy of a path name for access control. Note that currently any user that has access to the Nix daemon socket can discover all store paths by doing |
|
Thanks, the part about I get why you feel it has a "security through obscurity" vibe, since it does depend on hashes being secret. However, I think it's important to take into account that making this change won't worsen security, since |
|
I think the key concern is: If unix had always treated paths as a capability security mechanism kernel and software developers would design new interfaces and patches quite differently. With the current state of paths generally being considered mostly harmless to share with the entire system the risk of leaks is high, if not current interfaces then ones that may appear in the future. And without a good way to detect these leaks it is a very real threat to the security of sandboxes relying on "capability-based" Again, I don't think this is a blocker but I don't think this RFC is complete without thinking through the security risks that are present in this approach compared to options such as mounting only the needed paths that this RFC is suggesting could be replaced by this. |
…nix/store for other users
|
I made changes to the RFC about the drawbacks of depending on this for sandboxing and fixed some other things. |
spacekookie
added
the
status: open for nominations
|
Opening this RFC for nominations. Can nominated shepherds please respond whether they accept? |
…it on /nix/store for other users
|
I can be a shepherd. |
|
We still need one more shepherd here. |
|
@L-as do you want to comment on questions by @Profpatsch or maybe update some writing to improve readability? In particular, one thing maybe indeed we need to state clearly is that any changes in the name of implementing this RFC should create no new test failures (and if some VM tests somehow end up broken, the burden of fixing things or delaying the offending part is on the implementation PR) @edolstra what is your current opinion on the current state of the RFC? |
|
I feel that your response is good enough. Not breaking existing tests is I think not worth mentioning. Any change should of course not break tests, no matter what that change is. |
|
Not breaking existing tests is I think not worth mentioning. Any change should of course not break tests, no matter what that change is.
Well, there are cases where existing tests are declared reliant on obsolete assumptions and removed, or partially fixed with least important ones left to languish (roughly along the lines of RFC 0088).
This is not what should happen here (I think the RFC as-is gives nothing to lighten the burden of justification of removing tests; it could explicitly add to it but I also hope this bar won't be cleared either way).
Maybe the PR name should be «Allow unsetting…» now.
I preserve my agreement to move to FCP with disposition to accept.
|
|
@7c6f434c Yeah I'm 👍 on FCP/accept. |
spacekookie
added
status: FCP
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
How does this interact with https://github.com/edolstra/nix/commits/acls? |
|
@tomberek It shouldn't have an effect, AFAIK ACL paths will work fine in a non-world-readable store. |
| `/run/current-system`, and many other places which reveal your | ||
| system's closure, making this permission change an insufficient solution for | ||
| security in many cases. This, however, is also entirely optional and is not | ||
| the default in any way. |
There was a problem hiding this comment.
I assume that at least some filesystems are vulnerable to timing attacks, probing paths with stat to build prefixes of subdirectory paths until the full path is recovered; using the x bit to recover r for all paths, including ones that aren't running.
Nonetheless, it's good for practical hermeticity.
There was a problem hiding this comment.
Yes, we hope that people either look at the list and start, like you, name extra leak vectors, or trust «insufficient for security» claim. It would be interesting to have a survey of path leak vectors, but this RFC is shorter than any useful survey like that…
There was a problem hiding this comment.
I agree. Any list we try to create in this RFC is likely incomplete so it is better to just state that as an assumption and accept it for the rationale here. If someone wants to prove that the list of leaks is empty (and expected to stay that way) they can propose a follow-up RFC to rely on that fact.
| If a user on a non-NixOS platform mistakenly sets the permissions for `/nix/store` to | ||
| something undesirable, it won't be reverted by Nix automatically. |
There was a problem hiding this comment.
Why not restrict it to sensible or at least not terrible values? Seems quite feasible and the allow list could be extended if we discover a new use case.
There was a problem hiding this comment.
Because this «undesirable» includes the idea that most likely unfortunate mistakes are someon else's «just as planned» (I am not even sure 0000 is always useless; hmmm, now I am tempted to try once this is merged and trying is easy)
|
Has this ever been implemented? When I look at one of the lines mentioned in the RFC, it hasn't been changed since. |
| # Future work | ||
| [future]: #future-work | ||
|
|
||
| In the future we likely want to reduce the default permissions for `/nix/store` as much as possible. |
There was a problem hiding this comment.
Sometimes I use tab completion on a store path; saves some mousing when I fail to copy a path completely, or sometimes typing three or four letters of the hash is easy enough.
|
I realised it wasn't actually that useful due to 7c6f434c's comments about being able to query all paths known from Hydra. Also, logs leak paths. Slightly better security, but only slightly. |
I don't think an RFC was strictly necessary for this (potential?) change. It'd be fine to introduce it as an opt-in hardening option and then perhaps create the RFC to call attention to it? Maybe I don't see the whole history though. EDIT: simultaneous reply by L-as explains it. |
|
Yeah looking back it didn't need an RFC, but if anyone implements this at least it's decided that it will be merged. It's a very low cost change. |
infinisil
added
status: accepted
and removed
status: FCP
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/nixcon-governance-workshop/32705/9 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/musing-on-store-permissions/33695/2 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
This is effectively equal to
chmod o-r /nix/store.Rendered